Skip to content

fix(webui): Fix more memory leaks#1765

Merged
PeterC89 merged 9 commits into
Sofie-Automation:mainfrom
bbc:fix-memory-leaks
Jun 23, 2026
Merged

fix(webui): Fix more memory leaks#1765
PeterC89 merged 9 commits into
Sofie-Automation:mainfrom
bbc:fix-memory-leaks

Conversation

@PeterC89

@PeterC89 PeterC89 commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

About the Contributor

This pull request is posted on behalf of the BBC.

Type of Contribution

This is a:

Bug fix

Current Behaviour

UI eventually crashes due to running out of memory.

After running some long term observations / capturing heap snapshots etc. it appears that rundown segments can begin to pile up over time and don't get detached properly.

New Behaviour

Fewer detached DOM elements, there's still more than I would like but I'm not really sure how to proceed at this point as I can't tell where they're still being referenced.

Testing

  • I have added one or more unit tests for this PR
  • I have updated the relevant unit tests
  • No unit test changes are needed for this PR

Affected areas

This PR affects the webui

Time Frame

Not urgent, but we would like to get this merged into the in-development release.

Other Information

As far as I can tell the UI still works as expected and seems slightly more responsive, I imagine time will tell if this PR has made any different or not.

Copilot summarised this as:

  • Migrated preview popup rendering into portal flow and hardened popup/session lifecycle cleanup.
  • Added stale-session protection in preview context to prevent orphaned animation-frame checks and detached-anchor retention.
  • Reworked popup virtual anchor tracking to use stable ref-backed coordinates, reducing closure churn during mouse tracking.
  • Consolidated iframe preview load/postMessage handling with explicit listener cleanup.
  • Strengthened hover-preview cleanup paths (timeouts/session teardown on unmount and close).
  • Fixed unmount cleanup for timeline rendering helpers (including pending resize/debounce cancellation).

Status

  • PR is ready to be reviewed.
  • The functionality has been tested by the author.
  • Relevant unit tests has been added / updated.
  • Relevant documentation (code comments, system documentation) has been added / updated.

@PeterC89 PeterC89 added 🐛🔨 bugfix This is a fix for an issue Contribution from BBC Contributions sponsored by BBC (bbc.co.uk) labels Jun 4, 2026
@coderabbitai

coderabbitai Bot commented Jun 4, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

This PR hardens WebUI lifecycle behavior by implementing consistent cleanup patterns: tracking async work ownership (timers, RAF, observers), guarding against detached DOM operations, preventing state updates after unmount, and safely removing DOM-mounted elements. The changes span shared libs (observer pruning, portal cleanup, scroll state), the preview popup system (anchor tracking, session auto-close on disconnect), and timeline/rendering infrastructure (unmount guards, safe mount/remove helpers, timer tracking).

Changes

DOM & Lifecycle Cleanup Infrastructure

Layer / File(s) Summary
Observer management and portal cleanup
packages/webui/src/client/lib/VirtualElement.tsx, packages/webui/src/client/lib/Escape.tsx
VirtualElement's ElementObserverManager now prunes detached elements via document.contains, conditionally manages MutationObserver only when observed elements are connected, and tracks observed element identity via observedElementRef. Escape.tsx tracks whether parent was created by the hook and gates parent removal on teardown.
Scroll and viewport lifecycle
packages/webui/src/client/lib/viewPort.ts
Introduces clearPendingScrollState() to cancel pending scroll timeouts/idle callbacks; scroll flows clear state on retries, failures, and completion; focus/viewport teardown functions now reset scroll-tracking state including lastProgrammaticScrollTime.
Document class ownership utility
packages/webui/src/client/ui/util/useSetDocumentClass.ts
New useOwnedElementClassToggle hook removes/restores CSS class only when the same hook instance performed the removal, preventing interference from concurrent mount cycles.

Preview Popup Session Lifecycle Management

Layer / File(s) Summary
Preview popup anchor tracking and session lifecycle
packages/webui/src/client/ui/PreviewPopUp/PreviewPopUp.tsx, packages/webui/src/client/ui/PreviewPopUp/PreviewPopUpContext.tsx, packages/webui/src/client/ui/PreviewPopUp/Previews/IFramePreview.tsx
PreviewPopUp refactors to ref-driven virtual positioning with virtualPositionRef/anchorRef/anchorYRef, skips mouse movement and updates when anchor is detached. PreviewPopUpContext adds session tokens, auto-closes via RAF polling on anchor disconnection, returns no-op handles for already-detached anchors, and remounts popup per session via Escape wrapper and key prop. IFramePreview consolidates load listener into single effect with immediate invocation.
Preview session cleanup across UI consumers
packages/webui/src/client/ui/SegmentList/LinePartMainPiece/LinePartMainPiece.tsx, packages/webui/src/client/ui/SegmentList/LinePartPieceIndicator/LinePartAdLibIndicator.tsx, packages/webui/src/client/ui/SegmentList/LinePartPieceIndicator/LinePartScriptPiece.tsx, packages/webui/src/client/ui/SegmentList/LinePartSecondaryPiece/LinePartSecondaryPiece.tsx, packages/webui/src/client/ui/SegmentStoryboard/StoryboardPartSecondaryPieces/StoryboardSecondaryPiece.tsx, packages/webui/src/client/ui/SegmentStoryboard/StoryboardPartThumbnail/StoryboardPartThumbnailInner.tsx, packages/webui/src/client/ui/SegmentTimeline/Parts/InvalidPartCover.tsx, packages/webui/src/client/ui/Shelf/Renderers/L3rdListItemRenderer.tsx, packages/webui/src/client/ui/Shelf/Renderers/VTListItemRenderer.tsx, packages/webui/src/client/ui/Shelf/DashboardPieceButton/usePreviewPopUpSession.ts, packages/webui/src/client/ui/ClockView/CameraScreen/index.tsx
Components now close existing sessions before requesting new previews, use e.currentTarget as anchor instead of e.target, add useEffect unmount cleanup to close/clear session refs, and enforce connected HTMLElement anchors. usePreviewPopUpSession updates signature to require HTMLElement anchor and checks isConnected before preview startup; hover-timeout handler early-returns when unmounted.

Timeline & Renderer Lifecycle Management

Layer / File(s) Summary
Rundown view deferred callback tracking
packages/webui/src/client/ui/RundownView.tsx
Tracks idle callbacks and short/long timeout handles for "go to top" and "go to live" behaviors via dedicated instance fields; centralizes cancellation in clearPendingDeferredCallbacks() to prevent overlapping timers; calls viewport lifecycle reset on unmount.
Timeline container and part unmount guards
packages/webui/src/client/ui/SegmentTimeline/SegmentTimeline.tsx, packages/webui/src/client/ui/SegmentTimeline/SegmentTimelineContainer.tsx, packages/webui/src/client/ui/SegmentTimeline/Parts/SegmentTimelinePart.tsx
SegmentTimeline tracks and clears show-entire-segment timeouts, nulls DOM refs on unmount. SegmentTimelineContainer adds isUnmounted flag, stores timeout/RAF IDs for initial delay logic, cancels debounced visibility timers and throttled resize handlers on unmount, early-returns all event handlers when unmounted. SegmentTimelinePart props replace callback with data-driven collapsedOutputs map and isCollapsed flag.
Timeline renderer DOM helpers
packages/webui/src/client/ui/SegmentTimeline/Renderers/MicSourceRenderer.tsx, packages/webui/src/client/ui/SegmentTimeline/Renderers/VTSourceRenderer.tsx
Introduces safe mount/remove private helpers with try/catch and document.contains guards. VTSourceRenderer computes dedicated target-element helpers for right-label and countdown containers, guards portal creation with document.contains, tracks appendage state changes via targeted hasStateChanges check, and nulls container refs on unmount.
Timeline core item and layout cleanup
packages/webui/src/client/ui/SegmentTimeline/SourceLayerItem.tsx, packages/webui/src/client/ui/SegmentTimeline/TimelineGrid.tsx, packages/webui/src/client/ui/SegmentList/SegmentList.tsx
SourceLayerItem cancels cursor-position RAF on teardown, closes preview sessions, updates togglePreviewPopUp callback dependencies and anchor target usage. TimelineGrid cancels scheduled repaints and throttled resize handlers on unmount. SegmentList refactors detached-header scroll logic to compute threshold once and check element connectivity in scroll handler.

Event Handler Memoization & Listener Lifecycle

Layer / File(s) Summary
Wheel event handler memoization
packages/webui/src/client/ui/SegmentAdlibTesting/SegmentAdlibTesting.tsx, packages/webui/src/client/ui/SegmentStoryboard/SegmentStoryboard.tsx
Wheel handlers converted to useCallback with explicit dependencies; listener effects now depend on handler references instead of ref targets, ensuring correct re-binding when handler changes.
Menu and timer listener lifecycle
packages/webui/src/client/ui/SegmentList/LinePartPieceIndicator/LinePartIndicator.tsx, packages/webui/src/client/ui/SegmentList/LinePartPieceIndicator/LinePartAdLibIndicator.tsx, packages/webui/src/client/ui/SegmentList/LinePartPieceIndicator/PieceIndicatorMenu.tsx
LinePartIndicator attaches outside-click listener only while menu is open via conditional effect setup/cleanup. LinePartAdLibIndicator stores reveal timeout in ref with unmount cleanup. PieceIndicatorMenu gates mouseover/mouseleave setup on element connection and updates effect dependencies.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly Related PRs

  • Sofie-Automation/sofie-core#1760: Overlaps directly in PreviewPopUp/PreviewPopUpContext code, modifying session lifecycle and popup rendering logic within the same components.

Suggested reviewers

  • mint-dewit
  • nytamin
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 12.12% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title 'fix(webui): Fix more memory leaks' directly relates to the changeset which systematically addresses memory leaks across UI components through improved cleanup, lifecycle management, and DOM element detachment.
Description check ✅ Passed The PR description comprehensively covers the memory leak bug being fixed, the investigation that identified DOM element retention issues, and the technical improvements implemented to reduce detached elements and improve cleanup.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jun 4, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@PeterC89 PeterC89 force-pushed the fix-memory-leaks branch 2 times, most recently from 8e448db to fe66f7f Compare June 8, 2026 10:41
@PeterC89 PeterC89 force-pushed the fix-memory-leaks branch from fe66f7f to 99fd994 Compare June 8, 2026 10:57
@PeterC89

PeterC89 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
packages/webui/src/client/lib/viewPort.ts (1)

258-265: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Release currentScrollingElement on the non-second-stage exits.

Line 263 stores a live segment element, but the first-stage success path and the no-scroll return never clear it. If that segment is later detached, the module scope still keeps the node reachable until another scroll or lifecycle reset happens.

Suggested fix
 					if (!secondStage) {
 						//  Wait to settle 1 atemt to scroll
 						pendingFirstStageTimeout = setTimeout(() => {
 							pendingFirstStageTimeout = undefined
 							let { top, bottom } = elementToScrollTo.getBoundingClientRect()
 							top = Math.floor(top)
 							bottom = Math.floor(bottom)
 							if (bottom > Math.floor(window.innerHeight) || top < headerHeight) {
 								// If not in place atempt to scroll again
 								innerScrollToSegment(elementToScrollTo, forceScroll, true, true).then(resolve, reject)
 							} else {
+								currentScrollingElement = undefined
 								resolve(true)
 							}
 						}, 1000) // When UI is getting optimized further we could lower this value
 					} else {
 						currentScrollingElement = undefined
 						resolve(true)
 					}
 				})
 			},
@@
 	}
 
+	currentScrollingElement = undefined
 	return Promise.resolve(false)
 }

Also applies to: 287-299, 315-315

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/webui/src/client/lib/viewPort.ts` around lines 258 - 265,
currentScrollingElement is being retained on non-second-stage exits which keeps
DOM nodes reachable; change the logic so non-second-stage paths release the
reference by setting currentScrollingElement = undefined instead of storing
elementToScrollTo, and ensure you also clear currentScrollingElement before any
early/no-scroll returns and in the similar blocks referenced (the branches
around pendingFirstStageTimeout, the block covering lines ~287-299, and the
single-line exit near 315). Update places that set currentScrollingElement to
elementToScrollTo (and any early-return paths) to clear it (use
currentScrollingElement = undefined) and keep the second-stage behavior
unchanged.
packages/webui/src/client/ui/SegmentTimeline/SourceLayerItem.tsx (1)

253-260: ⚠️ Potential issue | 🟡 Minor

Add togglePreviewPopUp to the mini-inspector callback dependency arrays
toggleMiniInspectorOn/toggleMiniInspectorOff call togglePreviewPopUp but don’t depend on it, so they can retain a stale togglePreviewPopUp closure when its own dependencies change (also applies to ~333-342).

Suggested fix
 const toggleMiniInspectorOn = useCallback(
 	(e: React.MouseEvent) => togglePreviewPopUp(e, true),
-	[piece, cursorTimePosition, contentStatus, timeScale]
+	[togglePreviewPopUp]
 )
 const toggleMiniInspectorOff = useCallback(
 	(e: React.MouseEvent) => togglePreviewPopUp(e, false),
-	[piece, cursorTimePosition, contentStatus, timeScale]
+	[togglePreviewPopUp]
 )
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/webui/src/client/ui/SegmentTimeline/SourceLayerItem.tsx` around
lines 253 - 260, The two callbacks toggleMiniInspectorOn and
toggleMiniInspectorOff call togglePreviewPopUp but don't list it in their
dependency arrays, which can lead to stale closures; update both useCallback
dependency arrays to include togglePreviewPopUp, and also add togglePreviewPopUp
to the dependency arrays of the other callbacks in the same area (~lines
333-342) that invoke it so they always capture the latest togglePreviewPopUp
reference.
packages/webui/src/client/ui/SegmentTimeline/TimelineGrid.tsx (1)

444-452: ⚠️ Potential issue | 🟡 Minor

Cancel contextResize throttling during unmount

contextResize is a _.throttle that can execute a trailing call after componentWillUnmount, invoking repaint() / props.onResize(). Add this.contextResize.cancel() (same .cancel() pattern already used for other _.throttle handlers in this repo).

Suggested fix
 componentWillUnmount(): void {
 	if (typeof this.scheduledRepaint === 'number') {
 		window.cancelAnimationFrame(this.scheduledRepaint)
 		this.scheduledRepaint = null
 	}
+	this.contextResize.cancel()
 	if (this._resizeObserver) this._resizeObserver.disconnect()
 	window.removeEventListener(RundownTiming.Events.timeupdateLowResolution, this.onTimeupdate)
 	window.removeEventListener(RundownTiming.Events.timeupdateHighResolution, this.onTimeupdate)
 }
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/webui/src/client/ui/SegmentTimeline/TimelineGrid.tsx` around lines
444 - 452, componentWillUnmount must cancel the _.throttle stored on
this.contextResize to prevent its trailing invocation after unmount (which can
call repaint() / props.onResize()); update componentWillUnmount to call
this.contextResize.cancel() (following the same pattern used for other throttled
handlers) before unsetting observers/listeners so any queued trailing call is
cleared.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/webui/src/client/lib/Escape.tsx`:
- Around line 55-58: The cleanup currently sets rootElemRef.current = null which
breaks portals across id changes; update the cleanup inside the effect that uses
rootElemRef and parentCreatedByThisHook so it removes the element from the DOM
but does NOT null out rootElemRef.current on dependency changes, and instead
perform nulling/removal only on final unmount (e.g. move the nulling into a
separate useEffect with empty deps or guard it with a check that confirms the
component is unmounting), keeping rootElemRef alive across id changes so the new
effect can reuse the existing node; change references in Escape.tsx
(rootElemRef, removeElement, parentCreatedByThisHook, parentElem) accordingly.

In `@packages/webui/src/client/lib/viewPort.ts`:
- Around line 52-54: The code sets
viewPortScrollingState.isProgrammaticScrollInProgress and focusState.startTime
before knowing whether scrollToPartInstance will actually move the viewport;
move the mutation so it only happens after you determine a non-zero scroll will
occur and immediately before starting the smooth-scroll path. Specifically, in
the scroll-to logic (where focusState.startTime,
viewPortScrollingState.isProgrammaticScrollInProgress, and
viewPortScrollingState.lastProgrammaticScrollTime are currently set), check
whether the target requires movement, and only then set
isProgrammaticScrollInProgress and lastProgrammaticScrollTime; in the
already-visible and noAnimation/instant paths do not set the flag (or clear it
synchronously) so getViewPortScrollingState()/VirtualElement.isScrolling() does
not incorrectly report an active programmatic scroll.

---

Outside diff comments:
In `@packages/webui/src/client/lib/viewPort.ts`:
- Around line 258-265: currentScrollingElement is being retained on
non-second-stage exits which keeps DOM nodes reachable; change the logic so
non-second-stage paths release the reference by setting currentScrollingElement
= undefined instead of storing elementToScrollTo, and ensure you also clear
currentScrollingElement before any early/no-scroll returns and in the similar
blocks referenced (the branches around pendingFirstStageTimeout, the block
covering lines ~287-299, and the single-line exit near 315). Update places that
set currentScrollingElement to elementToScrollTo (and any early-return paths) to
clear it (use currentScrollingElement = undefined) and keep the second-stage
behavior unchanged.

In `@packages/webui/src/client/ui/SegmentTimeline/SourceLayerItem.tsx`:
- Around line 253-260: The two callbacks toggleMiniInspectorOn and
toggleMiniInspectorOff call togglePreviewPopUp but don't list it in their
dependency arrays, which can lead to stale closures; update both useCallback
dependency arrays to include togglePreviewPopUp, and also add togglePreviewPopUp
to the dependency arrays of the other callbacks in the same area (~lines
333-342) that invoke it so they always capture the latest togglePreviewPopUp
reference.

In `@packages/webui/src/client/ui/SegmentTimeline/TimelineGrid.tsx`:
- Around line 444-452: componentWillUnmount must cancel the _.throttle stored on
this.contextResize to prevent its trailing invocation after unmount (which can
call repaint() / props.onResize()); update componentWillUnmount to call
this.contextResize.cancel() (following the same pattern used for other throttled
handlers) before unsetting observers/listeners so any queued trailing call is
cleared.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2a13e384-1d7d-4797-8947-07c11ea4849c

📥 Commits

Reviewing files that changed from the base of the PR and between 3826017 and 99fd994.

📒 Files selected for processing (30)
  • packages/webui/src/client/lib/Escape.tsx
  • packages/webui/src/client/lib/VirtualElement.tsx
  • packages/webui/src/client/lib/viewPort.ts
  • packages/webui/src/client/ui/ClockView/CameraScreen/index.tsx
  • packages/webui/src/client/ui/PreviewPopUp/PreviewPopUp.tsx
  • packages/webui/src/client/ui/PreviewPopUp/PreviewPopUpContext.tsx
  • packages/webui/src/client/ui/RundownView.tsx
  • packages/webui/src/client/ui/SegmentAdlibTesting/SegmentAdlibTesting.tsx
  • packages/webui/src/client/ui/SegmentList/LinePartMainPiece/LinePartMainPiece.tsx
  • packages/webui/src/client/ui/SegmentList/LinePartPieceIndicator/LinePartAdLibIndicator.tsx
  • packages/webui/src/client/ui/SegmentList/LinePartPieceIndicator/LinePartIndicator.tsx
  • packages/webui/src/client/ui/SegmentList/LinePartPieceIndicator/LinePartScriptPiece.tsx
  • packages/webui/src/client/ui/SegmentList/LinePartPieceIndicator/PieceIndicatorMenu.tsx
  • packages/webui/src/client/ui/SegmentList/LinePartSecondaryPiece/LinePartSecondaryPiece.tsx
  • packages/webui/src/client/ui/SegmentList/SegmentList.tsx
  • packages/webui/src/client/ui/SegmentStoryboard/SegmentStoryboard.tsx
  • packages/webui/src/client/ui/SegmentStoryboard/StoryboardPartSecondaryPieces/StoryboardSecondaryPiece.tsx
  • packages/webui/src/client/ui/SegmentStoryboard/StoryboardPartThumbnail/StoryboardPartThumbnailInner.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Parts/InvalidPartCover.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Parts/SegmentTimelinePart.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/MicSourceRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/Renderers/VTSourceRenderer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/SegmentTimeline.tsx
  • packages/webui/src/client/ui/SegmentTimeline/SegmentTimelineContainer.tsx
  • packages/webui/src/client/ui/SegmentTimeline/SourceLayerItem.tsx
  • packages/webui/src/client/ui/SegmentTimeline/TimelineGrid.tsx
  • packages/webui/src/client/ui/Shelf/DashboardPieceButton/usePreviewPopUpSession.ts
  • packages/webui/src/client/ui/Shelf/Renderers/L3rdListItemRenderer.tsx
  • packages/webui/src/client/ui/Shelf/Renderers/VTListItemRenderer.tsx
  • packages/webui/src/client/ui/util/useSetDocumentClass.ts
💤 Files with no reviewable changes (1)
  • packages/webui/src/client/ui/SegmentTimeline/Parts/SegmentTimelinePart.tsx

Comment thread packages/webui/src/client/lib/Escape.tsx
Comment thread packages/webui/src/client/lib/viewPort.ts Outdated
@PeterC89

PeterC89 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@PeterC89

PeterC89 commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 8, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@PeterC89 PeterC89 marked this pull request as ready for review June 8, 2026 14:37
@PeterC89

Copy link
Copy Markdown
Contributor Author

Further testing suggests improvement so will look to merge this tomorrow as no one has commented

@PeterC89

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/webui/src/client/ui/PreviewPopUp/PreviewPopUp.tsx`:
- Around line 72-75: The ternary expression in the initialVirtualX assignment is
failing lint checks due to improper formatting of the condition. Wrap the
conditional expression (trackMouse && typeof initialOffsetX === 'number') in
parentheses to provide proper grouping that complies with the formatter
requirements. This ensures the ternary operator and its operands are correctly
grouped for the linter.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 65ec20a2-8782-4b15-a46c-90bfd9a767b4

📥 Commits

Reviewing files that changed from the base of the PR and between 6a4693c and c88241f.

📒 Files selected for processing (7)
  • packages/webui/src/client/ui/PreviewPopUp/PreviewPopUp.tsx
  • packages/webui/src/client/ui/PreviewPopUp/PreviewPopUpContext.tsx
  • packages/webui/src/client/ui/RundownView.tsx
  • packages/webui/src/client/ui/SegmentList/LinePartMainPiece/LinePartMainPiece.tsx
  • packages/webui/src/client/ui/SegmentStoryboard/StoryboardPartSecondaryPieces/StoryboardSecondaryPiece.tsx
  • packages/webui/src/client/ui/SegmentStoryboard/StoryboardPartThumbnail/StoryboardPartThumbnailInner.tsx
  • packages/webui/src/client/ui/Shelf/DashboardPieceButton/usePreviewPopUpSession.ts
🚧 Files skipped from review as they are similar to previous changes (6)
  • packages/webui/src/client/ui/SegmentStoryboard/StoryboardPartSecondaryPieces/StoryboardSecondaryPiece.tsx
  • packages/webui/src/client/ui/Shelf/DashboardPieceButton/usePreviewPopUpSession.ts
  • packages/webui/src/client/ui/SegmentStoryboard/StoryboardPartThumbnail/StoryboardPartThumbnailInner.tsx
  • packages/webui/src/client/ui/SegmentList/LinePartMainPiece/LinePartMainPiece.tsx
  • packages/webui/src/client/ui/RundownView.tsx
  • packages/webui/src/client/ui/PreviewPopUp/PreviewPopUpContext.tsx

Comment thread packages/webui/src/client/ui/PreviewPopUp/PreviewPopUp.tsx
@PeterC89 PeterC89 merged commit ecbae3a into Sofie-Automation:main Jun 23, 2026
22 of 23 checks passed
@PeterC89 PeterC89 deleted the fix-memory-leaks branch June 23, 2026 08:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🐛🔨 bugfix This is a fix for an issue Contribution from BBC Contributions sponsored by BBC (bbc.co.uk)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant